fix(task): isolate task configuration from focused provider state - #1085
fix(task): isolate task configuration from focused provider state#1085edelauna wants to merge 4 commits into
Conversation
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTask operations now use task-owned mode and API configuration for prompts, context handling, request setup, rate limiting, and retries. Provider profile-change listeners were removed. Submitted mode changes update task state immediately, with expanded isolation tests. ChangesTask configuration flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/task/Task.ts`:
- Line 3997: Update setupProviderProfileChangeListener and the
ProviderProfileChanged handling so only the task that explicitly selected the
changed profile updates its task-owned apiConfiguration; prevent focused-task
changes from overwriting unrelated tasks’ API handlers or rate limits. Add a
package-local unit test covering a ProviderProfileChanged event emitted while
this task is not focused and verify its API configuration remains unchanged.
- Around line 4036-4037: Update submitUserMessage to synchronize the task’s
_taskMode immediately after provider.setMode(mode) succeeds, so subsequent
getTaskMode() consumers use the selected mode. Add a package-local unit test
covering a mode switch through submitUserMessage followed by a request,
verifying the updated mode is used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a7d3c65b-e2c6-4cca-9569-2fa63e26d78d
📒 Files selected for processing (2)
src/core/task/Task.tssrc/core/task/__tests__/Task.spec.ts
8023403 to
59414bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/task/__tests__/Task.spec.ts`:
- Around line 530-571: Update the “only applies profile changes to the focused
task” test to dispose both Task instances after assertions, using their
dispose() methods before the test completes. Ensure the shared mockProvider no
longer retains the parent and child profile-change listeners for subsequent
tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e5779997-ce83-47d3-bd7e-71b8bbacbd0c
📒 Files selected for processing (2)
src/core/task/Task.tssrc/core/task/__tests__/Task.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/task/Task.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/task/Task.ts (1)
1510-1518: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPersist the selected provider profile on the task.
After
provider.setProviderProfile(providerProfile)succeeds, this code rebuildsapibut does not update_taskApiConfigName.saveClineMessages()then persists the prior profile in task metadata. A resumed task can restore the wrong profile.Set the task API config name from
newState.currentApiConfigName, withproviderProfileas a fallback. Add a regression test that switches profiles and verifies persisted task metadata.Proposed fix
const newState = await provider.getState() +this.setTaskApiConfigName(newState?.currentApiConfigName ?? providerProfile) if (newState?.apiConfiguration) { this.updateApiConfiguration(newState.apiConfiguration) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/task/Task.ts` around lines 1510 - 1518, Update the provider-profile handling after setProviderProfile in the task flow to assign _taskApiConfigName from newState.currentApiConfigName, falling back to providerProfile when unavailable, before persisting task metadata. Add a regression test that switches profiles and verifies saveClineMessages() persists the selected profile.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/core/task/Task.ts`:
- Around line 1510-1518: Update the provider-profile handling after
setProviderProfile in the task flow to assign _taskApiConfigName from
newState.currentApiConfigName, falling back to providerProfile when unavailable,
before persisting task metadata. Add a regression test that switches profiles
and verifies saveClineMessages() persists the selected profile.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7808404d-5caa-4a5b-8337-45277dac09dc
📒 Files selected for processing (2)
src/core/task/Task.tssrc/core/task/__tests__/Task.spec.ts
github-actions
Bot
added
the
awaiting-review
1 participant
Related GitHub Issue
References: #369 (PR 1 of 3)
Description
This is PR 1 of 3 for #369. It prevents a task's mode and API/profile configuration from leaking through shared focused-provider state, while preserving current single-concurrency behavior.
Tasknow readsmodefromgetTaskMode()and API configuration from its ownapiConfigurationin system-prompt generation, manual and forced context condensing, request construction, and rate-limit/retry-backoff handling. Provider state remains the source for genuinely shared settings such as custom modes, experiments, and profile thresholds.Profile-change notifications now update only the focused task, preventing a profile change from replacing another live task's API handler/configuration. This is a prerequisite for safe concurrent delegation work, without introducing fan-out scheduling, queues, e2e changes, persistence changes, or extension API changes.
Test Procedure
pnpm --dir src exec vitest core/task/__tests__/Task.spec.tspnpm --dir src exec eslint --prune-suppressions --max-warnings=0 core/task/Task.ts core/task/__tests__/Task.spec.tspnpm --dir src check-typespnpm --dir src lintThe focused unit coverage verifies task-local mode/API configuration in prompt and request paths, task-local rate limits during retry backoff, and that profile changes update only the focused task.
Pre-Submission Checklist
Visual Snapshots
Not applicable; this PR has no UI changes.
Videos (interaction / animation only)
Not applicable; this PR has no UI changes.
Documentation Updates
Additional Notes
No changeset included; changesets are managed separately.
Get in Touch
Summary by CodeRabbit